Skip to content

colr: make non-exhaustive, add unit tests#149

Open
bradh wants to merge 1 commit intokixelated:mainfrom
bradh:nclc_follow_up_2026-04-02
Open

colr: make non-exhaustive, add unit tests#149
bradh wants to merge 1 commit intokixelated:mainfrom
bradh:nclc_follow_up_2026-04-02

Conversation

@bradh
Copy link
Copy Markdown
Collaborator

@bradh bradh commented Apr 2, 2026

Follow-up for #110, to add tests and make colr non-exhaustive per @kixelated comment at #110 (comment)

@libark does this look OK to you?

@bradh bradh force-pushed the nclc_follow_up_2026-04-02 branch from 7d51f4c to 9fd4f91 Compare April 2, 2026 09:05
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac3cc648-4281-4d4a-967a-d8ee669731bb

📥 Commits

Reviewing files that changed from the base of the PR and between 7d51f4c and 9fd4f91.

📒 Files selected for processing (1)
  • src/moov/trak/mdia/minf/stbl/stsd/colr.rs

Walkthrough

The change annotates the public Colr enum in src/moov/trak/mdia/minf/stbl/stsd/colr.rs with #[non_exhaustive] and adds two unit tests for the Colr::Nclc variant: one decoding a colr/nclc payload into the enum and one encoding a Colr::Nclc value into the expected byte sequence. Total diff: +41/-0.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: making the Colr enum non-exhaustive and adding unit tests.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose as a follow-up to add tests and make the colr type non-exhaustive.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/moov/trak/mdia/minf/stbl/stsd/colr.rs`:
- Around line 189-193: The test function test_nclc_decode in colr.rs has array
literals missing trailing commas which causes cargo fmt --check to fail; update
the byte array ENCODED (and the other arrays mentioned around lines 211-214 and
216-220) to include trailing commas after the last elements so rustfmt passes
(locate the ENCODED array inside fn test_nclc_decode and the other similar test
arrays and add a trailing comma after the final byte in each array literal).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b20f239a-fa0e-402b-8309-a7d5dd8d7523

📥 Commits

Reviewing files that changed from the base of the PR and between 54e7e5a and 7d51f4c.

📒 Files selected for processing (1)
  • src/moov/trak/mdia/minf/stbl/stsd/colr.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant